Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent recompiling by properly specifying build.rs #1005

Merged
merged 2 commits into from
Feb 1, 2023

Conversation

rukai
Copy link
Member

@rukai rukai commented Jan 31, 2023

Wins

This is a decent improvement to our local development cycle.
Previously running cargo test after modifying a test took 5.5s to build on my machine, with this PR it takes 3s to build.
I expect the improvement to be even more on machines with lower core counts than mine.

Investigation

By looking at cargo test --no-run --timings before and after the PR we can see what changed:

Before:

After:

The build script i.e. build.rs was rerunning and this was forcing all of shotover-proxy to be rebuilt and any example binaries.
Afterwards only the test binary that we modified is being rebuilt.

Solution

The problem with our current setup is that unless instructed otherwise cargo will rerun the build.rs everytime a single file is changed in the package.
The shotover-proxy/build.rs is no longer needed, I assume the user of the env var it emits was moved into test-helpers or tokio-bin-process, so I just removed it.
The other remaining build.rs files have been changed to declare the env var that they take in so that cargo doesnt default to rerunning every time.
see: https://doc.rust-lang.org/cargo/reference/build-scripts.html#change-detection

Copy link
Member

@benbromhead benbromhead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@rukai rukai merged commit a6ea8be into shotover:main Feb 1, 2023
rukai added a commit to rukai/shotover-proxy that referenced this pull request Feb 2, 2023
rukai added a commit to rukai/shotover-proxy that referenced this pull request Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants